fix(form): a tabbed/sectioned modal keeps every tab's values (#2959, #2153) - #2987
Merged
Conversation
…2153) The explicit-`sections` path rendered one SchemaRenderer — one react-hook-form instance and one <form> element — PER section, all sharing `formId`. Two failures compounded: the footer submit (`form={formId}`) can only reach the FIRST form, so section 2+ never entered the payload; and in the tabbed variant Radix unmounted the inactive panel, destroying that tab's state outright. ModalForm (stacked + contentLayout:'tabbed') and TabbedForm now render ONE form for all sections, as ObjectForm/DrawerForm already do. Stacked sections use the inline `section-divider` header (which now also renders the section description); tabbed sections go through a new FormSchema.fieldTabs (+ defaultFieldTab, fieldTabsPosition) that the form renderer distributes into force-mounted Radix panels — CSS-hidden, not unmounted, because react-hook-form skips validation for unmounted fields, which is how a required field on an unopened tab sailed past the client and returned as a server 400. Validation now points at the tab: the first rejected field activates its tab and every tab holding one is marked on its trigger, for client rules and server `fields[]` alike. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…osition work
Two follow-ups found by running the tabbed form in a real browser (console
preview gallery):
- `fieldTabs` / `defaultFieldTab` / `fieldTabsPosition` bled through
`...formProps` onto the <form> element ("React does not recognize the
`fieldTabs` prop on a DOM element"), the same leak the neighbouring
destructure already guards `objectName` / `onDirtyChange` / `fields` against.
Added to that list, with a regression test that spreads the form node at the
top level the way the SDUI dispatch does.
- `fieldTabsPosition: 'bottom' | 'right'` positioned the strip with
`order-last`, which does nothing unless the parent is a flex container — the
root was only flex in the vertical case (a quirk carried over verbatim from
TabbedForm). The Tabs root is now always flex (`flex-col` when horizontal),
the strip keeps its content size via `self-start`, and `bottom` swaps its
margin to `mt-4`.
Re-verified in the browser: single form, panels force-mounted with the inactive
ones computed `display: none`, strip content-width above the panel, and no
console errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sync before merging so CI validates the real merge result — main picked up a @object-ui/types refactor (#2985) and two plugin-form wizard changes while this branch was in flight.
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2959 (and the explicit-sections half of #2153, which was closed with only the
fieldGroupsbranch fixed).The two compounding mechanisms
ModalForm's explicit-sectionspath rendered oneSchemaRenderer— one react-hook-form instance and one<form>element — per section, all sharing the sameformId:form={formId}) can only be associated with the first of those forms, so section 2+ never reached the payload (ModalForm 显式多 section 表单:每个 section 独立 <form>,提交时丢弃第 2+ 个 section 的值 #2153); andtabbedvariant Radix unmounted the inactive panel, destroying that tab's form state outright ([console 17.0.0-rc.0 @4a4829d0] Tabbed create/edit modal still loses non-active-tab values (#2153 closed but explicit-sections path unfixed) — Radix unmount destroys per-tab form state #2959).Reported flow (HotCRM, 3 tabs, required
descriptionon tab 3): fill tab 1 → submit → server 400description is required→ switch to tab 3, fill it → submit → the server now reportssubject; description; status; priorityall missing, because the second submit's body had lost every earlier value.TabbedForm(the full-page tabbed form) had the identical defect.The fix
Both hosts now render ONE form for all sections, the pattern
ObjectFormandDrawerFormhave shipped for a while:section-dividervirtual field carries each group's header (which now also renders the section'sdescription, so no authored copy is dropped), with per-section column density applied ascolSpanagainst the shared grid (Dashboard-level filters follow-ups: showcase, more examples, docs, spec pairing (#2576 / framework#2501) #2578 parity);FormSchema.fieldTabs(+defaultFieldTab,fieldTabsPosition) that the form renderer distributes into force-mounted Radix panels.Force-mounted matters twice: CSS-hidden rather than unmounted keeps the values, and keeps the validation — react-hook-form's
validateFieldreturns early for unmounted fields, which is exactly how a required field on a tab nobody opened sailed past the client and came back as a server 400.Validation feedback now points at the tab, as the issue asked: the first rejected field's tab is activated and every tab holding one is marked on its trigger (
data-error) — for client-side rules and serverfields[]rejections alike.The per-field render body was hoisted out of the
fields.map(...)JSX into a namedrenderFormFieldhelper (mechanically, via a scripted move) so a tab panel can place the same field row without a second form instance.git diff -wshows the real change is +282/-84.Verification
packages/plugin-form/src/sectionedFormValues.test.tsx(6) andpackages/components/src/renderers/form/__tests__/form-field-tabs.test.tsx(7) — all green.fieldTabscontract). They fail for the reported reasons, not incidentally.plugin-form(sectionColumns, modalFieldGroups, discardGuard, selectOptions, subformHosts) andcomponents/renderers/form(9 files, 59 tests).<form>, all three panels in the DOM, inactive panels computeddisplay: none, and a value typed on tab 1 survives a tab round-trip. Also compiled the console's real Tailwind entry through PostCSS to confirmdata-[state=inactive]:hiddenis actually emitted (.data-\[state\=inactive\]\:hidden[data-state="inactive"]{display:none}) — the one thing jsdom cannot prove.One test-expectation update:
sectionColumns.test.tsxcounted the multi-column grid's raw children (4); the section header is now an inline divider inside that same grid, so it counts:scope > [data-field]instead. The #2128 guard it exists for — fields are direct children of the grid, not a lone<form>— is unchanged.Not in scope
SplitFormhas the same per-section-form defect, but a single form cannot span twoResizablePanels without a different structural approach; it needs its own change. Flagged separately.The issue's second suspicion —
ModalForm.tsxstripping the payload throughcheckField(..., 'write')onceperms.isLoadedflips, withMePermissionsProviderfailing closed — is untouched here: it is an independent path, and with the form state fixed the reported symptom is fully explained by the two mechanisms above.🤖 Generated with Claude Code